Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add gas config to the client.toml #5020

Merged
merged 13 commits into from
May 11, 2023
Merged

Add gas config to the client.toml #5020

merged 13 commits into from
May 11, 2023

Conversation

hieuvubk
Copy link
Contributor

@hieuvubk hieuvubk commented Apr 28, 2023 •
edited
Loading

Closes: #4962

What is the purpose of the change

Save gas, gas-prices, gas-adjustment into client.toml. So user no more needs to pass gas flags in cmd, it will get data from config.

Brief Changelog

  • Overwrite ConfigCmd
  • Use OsmosisCustomClient instead of ClientConfig: the purpose is to integrate some of the fields mentioned above
  • The config fields will be set & get directly on the client.toml file

Flow

  • You can configure gas or any field with the command
    osmosisd config gas 1000000
  • Run tx cmd, no need to pass gas flag more. Its will get data that saved in client.toml
  • If user wanna change they config, use osmosisd config [key] [value]

@hieuvubk hieuvubk added T:ice-box V:state/compatible/backport State machine compatible PR, should be backported labels May 5, 2023
@hieuvubk hieuvubk marked this pull request as ready for review May 5, 2023 10:44
@faddat
Copy link
Member

faddat commented May 7, 2023

@hieuvubk can you please add a changelog to this pr?

faddat
faddat approved these changes May 7, 2023
View reviewed changes
Copy link
Member

@faddat faddat left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, once it's got a changelog!

heh, gotcha covered on it :)

@czarcas7ic
Copy link
Member

@hieuvubk has this been compiled and tested locally?

@hieuvubk
Copy link
Contributor Author

hieuvubk commented May 8, 2023

@hieuvubk has this been compiled and tested locally?

yes, I tested in my locally, can write & cmd can read data from client.toml.
One thing I wanna to ask, should we config fees cause I see all tx cmd need at least one fee denom.

@czarcas7ic
Copy link
Member

@hieuvubk yeah that would be great, please ping when that has been added and tested against mainnet and I will merge!

@hieuvubk
Copy link
Contributor Author

@czarcas7ic Fees added, this is my test tx against mainnet using osmosisd tx bank send from to 100000uosmo --from key without any flag.
https://www.mintscan.io/osmosis/txs/87887D790741A45E495E3C5AB3CD89E49AC1BB906994B820B7D54C68624A86C3

@czarcas7ic
Copy link
Member

@hieuvubk will merge as soon as you get sim to pass!

@hieuvubk
Copy link
Contributor Author

@czarcas7ic It passed after rerunning. Seem st wrong in the last run

@czarcas7ic
Copy link
Member

Nice work @hieuvubk , merging!

@czarcas7ic czarcas7ic merged commit 7d36a3f into main May 11, 2023
@czarcas7ic czarcas7ic deleted the gas_config branch May 11, 2023 01:45
pysel pushed a commit that referenced this pull request Jun 6, 2023
@hieuvubk @faddat @pysel
Add gas config to the client.toml (#5020)
fe0e887 
* Create config.go

fixing

* config cmd

* format

* fix template

* overwrite ReadFromClientConfig func: change ClientConfig => OsmosisCustomConfig

* format

* lint

* go work sync

* add gas configuration to client.toml

* add fees var

* format

---------

Co-authored-by: Jacob Gadikian <[email protected]>
@github-actions github-actions bot mentioned this pull request May 1, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@faddat faddat faddat approved these changes

@czarcas7ic czarcas7ic czarcas7ic approved these changes

@p0mvn p0mvn Awaiting requested review from p0mvn

@mattverse mattverse Awaiting requested review from mattverse

@nicolaslara nicolaslara Awaiting requested review from nicolaslara

Assignees
No one assigned
Labels
T:ice-box V:state/compatible/backport State machine compatible PR, should be backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CLI: Add gas config to the client.toml config file
3 participants
@hieuvubk @faddat @czarcas7ic